Skip to content

Conversation

@snipe
Copy link
Member

@snipe snipe commented Nov 6, 2025

Boy, this was a real pain to figure out. (Thanks for the rubber-ducking, @uberbrady).

For some users, adding over 995 or so users via the Admin Users > Groups form would cause permissions to break in very strange ways, and their additional users wouldn't get added.

This was due to a php.ini setting that would limit the number of form fields that PHP will accept - and instead of throwing an error, it unhelpfully just truncates the values:

max_input_vars = 1000
max_multipart_body_parts = 1000

The 1000 limit was why the number seemed to always be stuck at 995.

Because there's no way to set that value dynamically (outside of using request_parse_body(), which does not buffer to the PHP input stream and could cause other issues), and because of the way PHP truncates this stuff, we had to do some javascript trickery to force the selected IDs into a hidden field so that it only counts as one field, which we then decipher in the controllers. It's janky, but short of asking every user to twiddle their PHP settings, this was the only safe way to handle the problem.

This also makes the UI for the add/remove a little nicer, although I'm sure some folks will complain anyway. We could try to add a search filter to the lists as well in a different PR, to make it easier to find specific users within very large lists, but that's a problem for another time, I think.

There's one small glitch where the unselected "add these users" entries don't get saved (because they become unselected) if you click around a bunch - working on that.

Screen.Recording.2025-11-06.at.8.45.26.PM.mov

Fixes #18157

@snipe snipe changed the title Fixed #18157 - only partial information stored on group save if lower max_input_vars and/or max_multipart_body_parts Fixed #18157 - only partial information stored on group save if lower max_input_vars and/or max_multipart_body_parts Nov 6, 2025
Copy link
Member

@uberbrady uberbrady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was some ridiculous sleuthing that you had to do to track this down. Who knew that max_input_vars actually would be the thing that was biting us on this one! I'm going to want to click around on this one, because I'm a little worried about how loading up a gajillion users might mess up the DOM, and I'm worried that the loss of the user search function might affect how people use this feature. But once I've done that and am convinced that the UX works, I'll be happy to approve it! Thank you for figuring this out - I was starting to doubt my own sanity.

$groupPermissions = Helper::selectedPermissionsArray($permissions, $permissions);
$selectedPermissions = $request->old('permissions', $groupPermissions);

$users = \App\Models\User::get();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this be a problem if we had, say, 30,000 users (which does happen for some larger users/customers)?

And if we wanted to still do this I would prefer to use ::all() because it's clearer that you're grabbing everything.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. But this is what I had to do to get it working for now.

@snipe snipe merged commit 9f6a73b into develop Nov 6, 2025
7 of 8 checks passed
@snipe snipe deleted the fix-for-groups branch November 6, 2025 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants